Skip to content

fix(vehicle): clear_pin_to_drive_admin now clears PIN-to-Drive, not speed limit#86

Merged
Bre77 merged 3 commits into
mainfrom
fm/tfa-ble-pin-clear-verify
Jul 21, 2026
Merged

fix(vehicle): clear_pin_to_drive_admin now clears PIN-to-Drive, not speed limit#86
Bre77 merged 3 commits into
mainfrom
fm/tfa-ble-pin-clear-verify

Conversation

@Bre77

@Bre77 Bre77 commented Jul 21, 2026

Copy link
Copy Markdown
Member

Intent

Fix a live-verified, security-sensitive BLE bug: clear_pin_to_drive_admin (tesla_fleet_api/tesla/vehicle/commands.py) was documented as clearing PIN-to-Drive but actually built DrivingClearSpeedLimitPinAction - the Speed-Limit-Mode pin clear, a completely different vehicle feature. This was flagged as suspected (not yet proven) in an earlier cross-transport parity audit (capstone F1). The captain explicitly authorized a live, physical experiment on his own garaged vehicle tonight to verify it: set both a temporary PIN-to-Drive password and a temporary Speed-Limit-Mode PIN via BLE, fire clear_pin_to_drive_admin once, and observe the result via an independent cloud telemetry channel (Teslemetry Fleet Telemetry), with a mandatory both-pins-cleared cleanup contract regardless of outcome. The vehicle itself rejected the live call with reason 'speed_limit_mode_active' - a rejection reason meaningful only to Speed Limit Mode, not PIN-to-Drive - which is decisive, vehicle-confirmed proof of the mismapping. (Two earlier attempts this session were blocked/inconclusive on unrelated BLE transport flakiness - reconnect churn and transient GATT errors - before this held-BLE-session run got a clean, confirmed rejection from the car.) The fix changes the built action to VehicleControlResetPinToDriveAdminAction (the action that actually matches the method's own docstring and the cloud REST endpoint's semantics), keeping the pin parameter only for cross-transport signature parity with the cloud path (the correct action has no pin field) - this mirrors an existing documented pattern in this codebase for other cross-transport form gaps (e.g. set_scheduled_departure's dead preconditioning args). A second live finding surfaced during cleanup and is documented in the fix: the corrected action requires Tesla account/OAuth credentials, so calling it over raw local BLE signing (VehicleBluetooth, no OAuth session) will always raise TeslaFleetMessageFaultCommandRequiresAccountCredentials - this is intentional, correct, vehicle-enforced behavior (not a regression to avoid), since the same signed action relayed through the Fleet-API-authenticated VehicleSigned cloud path can supply that proof. Added regression tests: a BLE-mocked test asserting the corrected proto action is built and the old speed-limit action is not, a sibling regression test pinning that speed_limit_clear_pin still builds DrivingClearSpeedLimitPinAction (so the two commands stay decoupled going forward), and a cross-transport parity test documenting the now-accepted form gap (cloud still sends pin in the REST body, BLE ignores it). All local checks pass: ruff, pyright --strict, and the full pytest suite (371 tests).

What Changed

  • clear_pin_to_drive_admin (tesla_fleet_api/tesla/vehicle/commands.py) now delegates to the existing reset_pin_to_drive_admin, building the signed VehicleControlResetPinToDriveAdminAction, instead of the previously mismapped DrivingClearSpeedLimitPinAction (Speed-Limit-Mode's PIN clear, a different vehicle feature). The pin param is kept only for cross-transport signature parity with the cloud REST endpoint, since the corrected action has no pin field; the docstring documents this gap plus that the action requires Tesla account/OAuth credentials and will raise TeslaFleetMessageFaultCommandRequiresAccountCredentials over raw BLE signing.
  • Added regression tests: a BLE-mocked test asserting the corrected action is built (and the old speed-limit action is not) for clear_pin_to_drive_admin, a sibling test pinning that speed_limit_clear_pin still builds DrivingClearSpeedLimitPinAction, and a cross-transport parity test documenting the accepted form gap (cloud sends pin in the REST body, BLE ignores it).
  • Refreshed CLAUDE.md's cross-transport-parity note to reflect the fix instead of describing the mismapping as still open, and made a minor formatting fix in the new BLE test.

Risk Assessment

✅ Low: Both previously reported findings (duplicate action construction, stale CLAUDE.md/AGENTS.md parity note) are cleanly resolved with minimal, non-functional changes; clear_pin_to_drive_admin now delegates to reset_pin_to_drive_admin() and the doc note accurately reflects the fix, with no new issues introduced.

Testing

Ran the new BLE-mocked and cross-transport regression tests plus the full 371-test suite (all passing), and additionally drove clear_pin_to_drive_admin through the real mocked-BLE signed-command build/encrypt/decrypt path in a standalone script to directly observe the wire-level protobuf action: it now sends vehicleControlResetPinToDriveAdminAction (correct) rather than the old mismapped drivingClearSpeedLimitPinAction, while speed_limit_clear_pin still independently sends drivingClearSpeedLimitPinAction, confirming the fix and the decoupling the intent describes. No findings.

Evidence: CLI transcript: clear_pin_to_drive_admin wire-level protobuf action (after fix)

clear_pin_to_drive_admin(pin='1234') -> reply: {'response': {'result': True, 'reason': ''}} protobuf action actually sent over BLE: 'vehicleControlResetPinToDriveAdminAction' => CORRECT: builds the PIN-to-Drive admin reset action.

clear_pin_to_drive_admin(pin='1234') -> reply: {'response': {'result': True, 'reason': ''}}
protobuf action actually sent over BLE: 'vehicleControlResetPinToDriveAdminAction'
=> CORRECT: builds the PIN-to-Drive admin reset action.
Evidence: Demo script used to produce the transcript (drives the real mocked-BLE signed-command path)
"""Manual end-to-end demonstration of the clear_pin_to_drive_admin fix.

Drives the real Commands.clear_pin_to_drive_admin() call through the same
mocked-BLE harness the unit tests use (no real vehicle/GATT connection, but
the full signed-command build/encrypt/decrypt path runs for real) and prints
which protobuf action actually gets put on the wire - this is exactly what a
library consumer (e.g. Home Assistant's BLE integration) would experience
when calling this method.

Run at the base commit (871e449, "before") vs the target commit (eb5ce15,
"after") to see the mismapped action get corrected.
"""

import asyncio
import sys

sys.path.insert(0, "tests")

from ble_mocked_transport import (  # noqa: E402
    MockedBleTransportTestCase,
    decrypt_sent_command,
    infotainment_action_ok_reply,
)
from tesla_fleet_api.tesla.vehicle.proto.car_server_pb2 import Action  # noqa: E402


class Demo(MockedBleTransportTestCase):
    async def run(self) -> None:
        vehicle, send = self.make_vehicle()
        send.return_value = infotainment_action_ok_reply()

        result = await vehicle.clear_pin_to_drive_admin(pin="1234")

        sent_msg = send.await_args.args[0]
        plaintext = decrypt_sent_command(vehicle, sent_msg)
        action = Action.FromString(plaintext)
        built = action.vehicleAction.WhichOneof("vehicle_action_msg")

        print(f"clear_pin_to_drive_admin(pin='1234') -> reply: {result}")
        print(f"protobuf action actually sent over BLE: {built!r}")
        if built == "vehicleControlResetPinToDriveAdminAction":
            print("=> CORRECT: builds the PIN-to-Drive admin reset action.")
        elif built == "drivingClearSpeedLimitPinAction":
            print(
                "=> BUG: builds DrivingClearSpeedLimitPinAction, the "
                "Speed-Limit-Mode pin clear - a different feature entirely."
            )
        else:
            print(f"=> UNEXPECTED action: {built!r}")


async def main() -> None:
    demo = Demo()
    await demo.asyncSetUp()
    try:
        await demo.run()
    finally:
        await demo.asyncTearDown()


if __name__ == "__main__":
    asyncio.run(main())

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

🔧 **Review** - 2 issues found → auto-fixed ✅
  • ⚠️ CLAUDE.md:142 - CLAUDE.md's cross-transport-parity entry (line 142) still describes clear_pin_to_drive_admin's speed-limit mismapping as 'One open divergence left unfixed pending live verification', but this PR fixes exactly that divergence and also introduces a new, now-accepted form gap (BLE ignores pin, cloud still sends it) that isn't reflected there either. A future agent relying on this doc will think the bug is still open.
  • ℹ️ tesla_fleet_api/tesla/vehicle/commands.py:1009 - clear_pin_to_drive_admin (line 990) now builds the exact same empty VehicleControlResetPinToDriveAdminAction as the pre-existing reset_pin_to_drive_admin (line 2366/2371), making them duplicate implementations of the same signed command with no shared code path. Not a functional bug, but a simplification opportunity (e.g. have one delegate to the other).

🔧 Fix: Dedupe pin-reset action and refresh stale CLAUDE.md parity note
✅ Re-checked - no issues remain.

✅ **Test** - passed

✅ No issues found.

  • uv run pytest tests/test_ble_mocked_commands.py -k "ClearPinToDriveAdmin or SpeedLimitClearPin" — 3 passed
  • uv run pytest tests/test_cross_transport_parity.py -k ClearPinToDriveAdmin — 1 passed
  • uv run pytest tests — full suite, 371 passed
  • Manual CLI demo: drove vehicle.clear_pin_to_drive_admin(pin='1234') through the mocked-BLE signed-command harness (real sign/encrypt/decrypt path, no live vehicle) and decoded the actual protobuf Action placed on the wire
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

firstmate crewmate added 3 commits July 21, 2026 20:00
…peed limit

Live-verified over BLE: the command built DrivingClearSpeedLimitPinAction
(the Speed-Limit-Mode pin clear) instead of a PIN-to-Drive action - the
vehicle itself rejected a live call with reason "speed_limit_mode_active",
a condition meaningful only to Speed Limit Mode. Now builds the correct
VehicleControlResetPinToDriveAdminAction, matching the method's own
docstring and the cloud REST endpoint's semantics. That action has no pin
field and requires Tesla account credentials, so it always fails over raw
BLE signing with TeslaFleetMessageFaultCommandRequiresAccountCredentials -
expected, not a regression (points callers at the Fleet-API-relayed path).
@Bre77
Bre77 merged commit f686c92 into main Jul 21, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant